Skip to content

add "cargo xtask local" to run buildomat locally#91

Open
emilyalbini wants to merge 1 commit intomainfrom
ea-kwvtkkzqoztv
Open

add "cargo xtask local" to run buildomat locally#91
emilyalbini wants to merge 1 commit intomainfrom
ea-kwvtkkzqoztv

Conversation

@emilyalbini
Copy link
Copy Markdown
Member

This PR adds the cargo xtask local family of commands to run a local instance of buildomat with minimal configuration.

The centerpiece if the cargo xtask local setup command, backed by the xtask-setup crate (it's split from xtask as it requires a lot of dependencies). It currently supports configuring buildomat-server, buildomat-github-server, and buildomat-factory-aws, and at startup asks which components the user wants to be configured.

There are then the cargo xtask local BINARY commands, which invokes the binary with the flags required to use the local setup. cargo xtask local buildomat also gives access to the CLI, pointed to the local server.

Other changes included in the PR:

  • Added a BUILDOMAT_CONFIG environment variable to the buildomat CLI to point to a separate configuration file, so that cargo xtask local buildomat can transparently use the correct credentials.
  • Made specifying the SSH key optional in the AWS factory.
  • Added the ability to allocate public IPs to instances in the AWS factory.

@emilyalbini
Copy link
Copy Markdown
Member Author

Rebased to fix merge conflicts.

pub key: Option<String>,
pub security_group: String,
pub limit_total: usize,
#[serde(default = "default_false")]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think false is already the Default::default() value for bool isn't it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, while that is correct, I personally prefer to add an explicit default value rather than deferring to Default::default. That way it's obvious what the default value is by glancing at the source code.

If you prefer to replace it with #[serde(default)] I can do the switch.

Comment on lines +383 to +401
* We are intentionally building the x86_64-unknown-linux-musl variant of
* the agent. Linux is what the AWS factory configures to run, and the musl
* variant prevents issues with older glibcs or building on NixOS.
*/
eprintln!("building the agent...");
let status = cargo()
.args(["build", "--bin", "buildomat-agent"])
.arg("--release")
.arg("--target=x86_64-unknown-linux-musl")
.status()?;
if !status.success() {
std::process::exit(status.code().unwrap_or(1));
}
std::fs::copy(
local
.join("../..")
.join("target/x86_64-unknown-linux-musl/release/buildomat-agent"),
local.join("buildomat-agent-linux"),
)?;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better if we built the illumos buildomat-agent binary here rather than the Linux one, as that's generally the primary focus here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change it to also build the illumos one if the current host is illumos, but last time I tried creating a cross-compilation environment from NixOS to illumos I didn't have much success. A Linux MUSL build AFAIK can be done from either Linux or illumos.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now xtask always builds the Linux MUSL agent (as the AWS runner configured by xtask-setup uses a Linux image), and if the illumos target is installed for rustc it also builds the illumos agent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants